-
Notifications
You must be signed in to change notification settings - Fork 879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support instrumentation for jsonrpc4j #13008
base: main
Are you sure you want to change the base?
feat: support instrumentation for jsonrpc4j #13008
Conversation
55628cf
to
b605e52
Compare
b605e52
to
0190c13
Compare
Hello @trask , could you please take a look when you have time? I'm also working on the testing. |
...etry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonServiceExporterBuilderInstrumentation.java
Outdated
Show resolved
Hide resolved
tasks { | ||
test { | ||
systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean) | ||
jvmArgs("-Dotel.javaagent.experimental.thread-propagation-debugger.enabled=false") | ||
jvmArgs("-Dotel.instrumentation.common.experimental.controller-telemetry.enabled=true") | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to keep jvmArgs("-Dotel.javaagent.experimental.thread-propagation-debugger.enabled=false")
, otherwise the test would fail due to "detect context leak".
the other two settings can be removed.
...ntelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcServerBuilderInstrumentation.java
Outdated
Show resolved
Hide resolved
...ntelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcClientBuilderInstrumentation.java
Outdated
Show resolved
Hide resolved
...ntelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcClientBuilderInstrumentation.java
Outdated
Show resolved
Hide resolved
...ntelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcClientBuilderInstrumentation.java
Outdated
Show resolved
Hide resolved
ee68060
to
6f50140
Compare
.../io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/JsonRpcServerInstrumentation.java
Outdated
Show resolved
Hide resolved
...rc/test/java/io/opentelemetry/javaagent/instrumentation/jsonrpc4j/v1_6/AgentJsonRpcTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forget to add related information in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md#libraries--frameworks
8e9b228
to
a54e28c
Compare
To resolve: #6020
The instrumentation is for both server and client:
JsonRpcServer
'sinvocationListener
as an instance ofOpenTelemetryJsonRpcInvocationListener
invoke
method of bothJsonRpcHttpClient
andJsonRpcRestClient
I'm new to this repository, so I borrowed the implementation of grpc and http client instrumentation